NetNews Offline 2/NetNews Offline Volume 2.iso/news/comp/lang/c-part1/3437

Text File | 1996-08-05 | 3.5 KB | 127 lines

Path: mail2news.demon.co.uk!genesis.demon.co.uk
From: Lawrence Kirby <[e-mail]>
Newsgroups: comp.lang.c
Subject: Re: Simple C program to list contents of a file - help!
Date: Sun, 28 Jan 96 23:15:50 GMT
Organization: none
Message-ID: <[e-mail]>
References: <4egks8$[e-mail]>
Reply-To: [e-mail]
X-NNTP-Posting-Host: genesis.demon.co.uk
X-Newsreader: Demon Internet Simple News v1.27
X-Mail2News-Path: genesis.demon.co.uk

In article <4egks8$[e-mail]>
           [e-mail] "Raghu R. Gompa" writes:

>
>I am trying to figure out what is wrong with this
>program.  The following is written for dos environment:
>"typeme.c"
>
>It is supposed to get words from file and list them with
>arguments like help me please with the command:
>     typeme -f file help me please
>
>Depending on the number of arguments, I sometimes getting
>strange error messages - system stuck some times.
>Please help.  This is part of a project (not a homework)
>I am trying to complete. .. Thanks in advance. .. Raghu
>
>

>#include "stdio.h"
>#include "string.h"

That should be

#include <stdio.h>
#include <string.h>

>FILE *fopen(), *fp;
>extern exit();

Don't try to declare standard functions yourself - that is what the
header files are for. For exit() you need:

#include <stdlib.h>

>main(int argc, char *argv[])
>{
>char first[100], lett, *words[100], filename[15];
>int numwords, cont=1;
>int i, j, repeat=10, r,k=0,t;
>int nrem;
>if(*argv[1] == '-') {
>    strcpy(filename, argv[2]);
>    fp= fopen(filename,"r");
>    if(fp == NULL) printf("Can't open %s \n",filename);

It is good that you are testing for failure. Unfortunately when it does fail
you are falling through to the subsequent code which will then try to pass
a null pointer to fscanf and so on which is illegal. If you have nothing
better to do here exit the program with, say, exit(EXIT_FAILURE);

>    k=0;*words[1]='1';

You have defined words as an array of 100 (uninitialised) pointers. Here
you are truing to dereference one of those unilitialised pointers which
is illegal. You need to set each pointer to point to something before
dereferencing it e.g. by using malloc.

>    while(fscanf(fp, "%s", words[k]) != EOF ) {

The same thing applies here.

>    k++;

You should test for this overflowing your array (>= 100 in this case).
However it is better not to use 'magic numbers' through the program but
rather (for example) #define a constant and us that throughout. That way
if you need to change the value you can do it at one point and you're
much less likely to have problems with the code due to oversights.

>    }
>    fclose(fp);
>    numwords=k-1;
>    if(argc>2) {
>          for(i=3;i<=argc;i++) {

You're going one too far here.

argv[0] is the program name
argv[1] to argv[argc-1] are the arguments
argv[argc] is a null pointer.

So in effect argc counts the program name as 1 argument.

>             strcpy(words[k],argv[i]);
>             k++;
>             }
>          }
>    numwords=k-1;
>   }
>else
>   {
>   numwords=argc-1;
>    for(k=0;k<numwords;k++)
>          strcpy(words[k],argv[k+1]);

Same pointer and range problems here.

>   }
>
>   nrem=1;
>   for(i=0;i< numwords;i++) {
>     printf("%3d. %10s", i+1, words[i]);
>     nrem=(i%4);
>     if(i>3 && !nrem) printf("\n");
>     }
>}

--
-----------------------------------------
Lawrence Kirby | [e-mail]
Wilts, England | [e-mail]
-----------------------------------------